Place multiple bids on a name in same block #477
Conversation
rithvikvibhu
added
the
needs translation
|
Do you think we should alert the user if they are placing a duplicate bid? |
|
I initially thought that the big blue "placed bid" modal and the button changing text is enough, but going through the video again, yeah an explicit warning might be better. |
rithvikvibhu
force-pushed
the
multi-bid-in-same-block
branch
from
e95c1b2 to
ca5a0cf
Compare
|
@pinheadmz added that warning when one tries to bid again with the same value: |
|
might be a nice tweak to also extend the "another bid" copy change to all the other submit/confirm parts of the UI, not just the first button. it would be easy to open the dialogue, get distracted, then come back and forget that the open prompt is for an extra bid, especially if the first bid's state may have changed in a way the user was unsure about in the meantime. |
| map[bid.from] = { | ||
| date: createAMPMTimeStamp(bid.date), | ||
| date: bid.date ? `${month}/${day}/${year}` : '(pending)', |
There was a problem hiding this comment.
we'll probably have to revisit date/time formatting in I18n friendly way at some point. not important to focus on here, just observing
app/pages/Auction/index.js
Outdated
| refreshInfo = throttle(async () => { | ||
| await this.props.getNameInfo(this.getDomain()); | ||
| await this.props.fetchPendingTransactions(); | ||
| }, 10*1000, {leading: true}) // 10 seconds |
There was a problem hiding this comment.
is this throttle rate based on some timeout defined elsewhere, or just an arbitrary rate?
does throttle handle the situation where the async function runs longer than 10 seconds? i.e. if it took 60 seconds to complete, would it fire up to 6-ish concurrent attempts, or would it wait for the first one to resolve before allowing the second to begin?
also, if i'm reading correctly, given the componentDidUpdate is async, and this function awaits on each line, is the async inside of throttle actually providing any benefit?
There was a problem hiding this comment.
Good points. 10 seconds just seemed enough, since fetchPendingTransactions() is a local call and getNameInfo is small json and fixed in size (if local or even if remote over network).
I still don't like the idea of random numbers like this, but without a throttle, it will refresh every time the height changes (which can happen more than once a second when syncing). Open to other cleaner ideas.
given the componentDidUpdate is async, and this function awaits on each line
You're right, we don't really care when it finishes and 118 doesn't need 117 to finish first, so removed await (and async) so they can run in parallel.
rithvikvibhu
force-pushed
the
multi-bid-in-same-block
branch
from
ca5a0cf to
c88f5d3
Compare
rithvikvibhu
force-pushed
the
multi-bid-in-same-block
branch
from
c88f5d3 to
51e7800
Compare
rithvikvibhu
removed
the
needs translation
Closes #179.
Demo (https://youtu.be/FNT78BhzhS4):
Changes made to make this happen:
getBlindtowalletClientBidHistorycomponent to show pending bids differently (gray text, italics)reducePendingTransactionsfromnamesReducer.jstowalletActions.jsbecause it no longer "just" reduces, it also calls async function getBlind. It anyway did a lot more than a reducer should before also.Also:
Recommend squash and merge because commits aren't really separate.